Skip to content

Dev - Volha#9

Open
Vuictorovna wants to merge 8 commits intoalexnaylor99:mainfrom
Vuictorovna:dev
Open

Dev - Volha#9
Vuictorovna wants to merge 8 commits intoalexnaylor99:mainfrom
Vuictorovna:dev

Conversation

@Vuictorovna
Copy link
Collaborator

I've added real-time stock monitoring features and IFTTT alerts to the project. Check it out when you get a chance!

What's I've done:

  • Real-time stock price checks
  • IFTTT notifications
  • Added comments and updated README

Looking forward to your thoughts!

@Vuictorovna Vuictorovna changed the title Dev Dev - Volha Aug 25, 2023
Copy link
Collaborator

@T-meji T-meji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great work! I like that everything is clear and well documented, as well as how you compartmentalised different aspects of your code, e.g., a separate webhooks file, a readme instruction manual for the app, etc.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great:

  • Notation/documentation
  • Clear
  • Concise
  • Unpacked
  • Kept to coding standards
  • Zen code
  • Kept API Key hidden (good for security reasons)

Areas for Improvement:

  • Add explanatory comments to lines 1, 12 and 78?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great:

  • Hid Webhooks key (good for security reasons)
  • Clear and concise
  • Zen and unpacked (kept to the one line rule)
  • Comments explain the corresponding code
  • Complex but not complicated
  • Met all brief objectives

Areas for Improvement:

  • Could added 'importing packages' comment on the first line?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great readme! A step-by-step guide for the stakeholders to ensure the application runs smoothly and that they can receive the IFTTT alerts. Very helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants